Skip to content

[SC-17187] ZD-717: GINITable fails on multiclass models (follow-up to PR #535)#539

Merged
juanmleng merged 2 commits into
mainfrom
juan/sc-17187/ginitable-fails-on-multiclass-models-follow-up-to
Jul 15, 2026
Merged

[SC-17187] ZD-717: GINITable fails on multiclass models (follow-up to PR #535)#539
juanmleng merged 2 commits into
mainfrom
juan/sc-17187/ginitable-fails-on-multiclass-models-follow-up-to

Conversation

@juanmleng

Copy link
Copy Markdown
Contributor

Pull Request Description

What and why?

GINITable (validmind/tests/model_validation/statsmodels/GINITable.py) computed AUC/GINI/KS with binary-only sklearn calls (roc_curve, roc_auc_score) against a single positive-class probability column. On a model with more than two classes it raised ValueError: multiclass format is not supported.

This adds a one-vs-rest path for multiclass models, mirroring the approach PR #535 used for the sklearn tests (ROCCurve, PrecisionRecallCurve, ConfusionMatrix, PopulationStabilityIndex). #535 fixed those four but did not touch statsmodels.GINITable; this is the remaining follow-up.

After this change:

  • Binary models: unchanged — one row of AUC/GINI/KS.
  • Multiclass models: the metrics are computed one-vs-rest and the table returns one row per class plus a micro-average row (with a Class column). Per-class probabilities are read from the underlying estimator's predict_proba, since the VMModel wrapper exposes only the positive-class column.
  • Models that can't produce a full per-class probability matrix (metadata-only models, or predictions supplied as a single precomputed probability column) raise SkipTestError instead of crashing.

How to test

Run the existing unit tests:

python -m pytest tests/unit_tests/model_validation/statsmodels/test_GINITable.py

Or reproduce the original failure and confirm the fix directly (this is the snippet from the repro notebook used to validate the change):

import numpy as np
import pandas as pd
from sklearn.datasets import make_classification
from sklearn.linear_model import LogisticRegression
import validmind as vm
from validmind.tests.model_validation.statsmodels.GINITable import GINITable


def build(n_classes, seed=42):
    X, y = make_classification(
        n_samples=300, n_features=5, n_informative=4, n_redundant=0,
        n_classes=n_classes, n_clusters_per_class=1, random_state=seed,
    )
    df = pd.DataFrame(X, columns=[f"f{i}" for i in range(5)])
    df["target"] = y
    model = LogisticRegression(max_iter=1000).fit(X, y)
    ds = vm.init_dataset(input_id=f"ds{n_classes}", dataset=df, target_column="target", __log=False)
    m = vm.init_model(input_id=f"m{n_classes}", model=model, __log=False)
    ds.assign_predictions(m)
    return ds, m


# Binary — worked before, still works
ds2, m2 = build(2)
print(GINITable(ds2, m2)[0])

# Multiclass (3 classes) — raised "ValueError: multiclass format is not supported"
# before this PR; now returns one row per class plus a micro-average
ds3, m3 = build(3)
print(GINITable(ds3, m3)[0])

Expected multiclass output:

   Class       AUC      GINI        KS
0      0  0.958650  0.917300  0.845000
1      1  0.926300  0.852600  0.725000
2      2  0.933950  0.867900  0.785000
3  micro  0.940167  0.880333  0.768333

What needs special review?

The multiclass path reads probabilities from the underlying estimator (model.model.predict_proba) rather than the VMModel wrapper's y_prob, because the wrapper returns only the positive-class column. Worth confirming this is the right access pattern (it matches ROCCurve from #535) and that the SkipTestError fallback covers the model types you expect (metadata-only / precomputed single-column probabilities).

Dependencies, breaking changes, and deployment notes

Follow-up to #535. No breaking changes — binary output is unchanged.

Release notes

GINITable now supports multiclass classification models. For models with more than two classes it reports AUC, GINI, and KS one-vs-rest (one row per class plus a micro-average) instead of failing with multiclass format is not supported. Binary models are unaffected.

Checklist

  • What and why
  • Screenshots or videos (Frontend)
  • How to test
  • What needs special review
  • Dependencies, breaking changes, and deployment notes
  • Labels applied
  • PR linked to Shortcut
  • Unit tests added (Backend)
  • Tested locally
  • Documentation updated (if required)
  • Environment variable additions/changes documented (if required)

GINITable computed AUC/GINI/KS with binary-only sklearn calls against a
single positive-class probability column, raising "multiclass format is
not supported" on models with more than two classes.

Add a one-vs-rest path that reads the full per-class probability matrix
from the underlying estimator's predict_proba, one-hot encodes labels with
label_binarize, and returns one row per class plus a micro-average. Binary
behavior is unchanged; models without a usable predict_proba are skipped
with SkipTestError instead of crashing.

Add multiclass unit tests covering the per-class table and the skip path.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@juanmleng juanmleng marked this pull request as ready for review July 15, 2026 13:47
@github-actions

Copy link
Copy Markdown
Contributor

Pull requests must include at least one of the required labels: internal (no release notes required), highlight, enhancement, bug, deprecation, documentation. Except for internal, pull requests must also include a description in the release notes section.

@juanmleng juanmleng self-assigned this Jul 15, 2026
@juanmleng juanmleng added the bug Something isn't working label Jul 15, 2026
@juanmleng juanmleng requested a review from cachafla July 15, 2026 13:48
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
Contributor

PR Summary

This PR extends the functionality of the GINITable module by adding support for multiclass classification metrics. The key changes include:

  • Introducing a new internal function _multiclass_gini_table which computes AUC, GINI, and KS metrics for multiclass models using a one-vs-rest approach. The function obtains the full per-class probability matrix from the underlying model (via predict_proba) and verifies that the returned shape matches the number of classes.
  • Updating the main GINITable function to delegate to the new multiclass logic when the dataset contains more than two unique classes.
  • Implementing robust error handling: if the underlying model does not support predict_proba or if the output shape is not as expected, a SkipTestError is raised to gracefully bypass unsupported scenarios.
  • Adding a comprehensive set of unit tests within the TestGINITableMulticlass class. These tests cover both successful metric computation (checking that per-class and micro-average metrics are correctly calculated and meet expected relationships) and the proper triggering of the SkipTestError when a model cannot produce per-class probabilities.

Overall, these changes improve the library's capability to handle multiclass model evaluations while maintaining backward compatibility with binary classification scenarios.

Test Suggestions

  • Test models with binary classification to ensure existing functionality remains unchanged.
  • Simulate a multiclass scenario where the model's predict_proba returns an improperly shaped array to trigger the SkipTestError.
  • Test models with a predict_proba method that raises an exception to verify that the error handling in _multiclass_gini_table works as expected.
  • Validate that the per-class and micro-average metrics, including AUC, GINI, and KS, are computed correctly for diverse datasets.

@juanmleng juanmleng requested a review from hunner July 15, 2026 14:57
@juanmleng juanmleng added enhancement New feature or request and removed bug Something isn't working labels Jul 15, 2026
@juanmleng juanmleng merged commit a347283 into main Jul 15, 2026
23 checks passed
@juanmleng juanmleng deleted the juan/sc-17187/ginitable-fails-on-multiclass-models-follow-up-to branch July 15, 2026 18:59

@hunner hunner left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving — the fix is correct and directly resolves the remaining failure in ZD-717 (GINITable on multiclass models). Verified locally on the PR branch: all 6 unit tests pass (4 existing binary + 2 new multiclass), and I probed the eval-dataset-missing-a-training-class case — the shape guard raises a clear SkipTestError rather than silently mislabeling curves. Binary path is unchanged, and the implementation faithfully mirrors #535's _multiclass_roc_curve.

Left three non-blocking suggestions inline (shared helper extraction, model.classes_ alignment, and one extra test) — all fine as follow-ups.

)


def _multiclass_gini_table(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Non-blocking / follow-up: this preamble (raw-estimator access → SkipTestError fallbacks → shape check → label_binarize) is now the 5th copy of the same block, alongside ROCCurve, PrecisionRecallCurve, ConfusionMatrix, and PopulationStabilityIndex from #535. The repo already extracts shared helpers for this kind of thing (see 0a80b55 "share diagnosis metric helpers") — a get_multiclass_proba_matrix(model, dataset, classes) helper would collapse all five call sites and give one place to fix alignment/skip behavior. Keeping this PR consistent with #535 and extracting in a follow-up across all five seems right.

- The test does not incorporate a method to efficiently handle missing or inefficiently processed data, which could
lead to inaccuracies in the metrics if the data is not appropriately preprocessed.
"""
classes = np.unique(dataset.y)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Non-blocking / follow-up: np.unique(dataset.y) is assumed to match the predict_proba column order, but sklearn orders columns by estimator.classes_ — the classes seen in training. For the ZD-717 customer's exact usage (input_grid over [vm_train_ds, vm_test_ds]), a rare class absent from the test split makes this skip on test while succeeding on train (I reproduced this: 4-class model, 3-class eval slice → SkipTestError from the shape guard). Safe, but a false negative — aligning on getattr(raw_model, "classes_", None) with np.unique as fallback would still compute in that case. Same inherited behavior in the four #535 tests, so best fixed once in the shared helper.

self.assertEqual(set(raw_data.fpr), {"0", "1", "2", "micro"})
self.assertEqual(set(raw_data.tpr), {"0", "1", "2", "micro"})

def test_multiclass_without_predict_proba_is_skipped(self):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Non-blocking: there's coverage for predict_proba being unavailable, but not for the other skip branch — a probability matrix whose column count doesn't match the dataset's classes (e.g. model trained on 4 classes, eval dataset containing only 3). That's the branch multiclass users are most likely to hit in practice via train/test input_grid, and it's cheap to add: fit on 4 classes, build the VM dataset from a slice with y != 3, assert SkipTestError.

hunner added a commit that referenced this pull request Jul 16, 2026
…(ZD-717)

Follow-ups from the PR #539 review. ROCCurve, PrecisionRecallCurve,
PopulationStabilityIndex (#535) and GINITable (#539) each carried an
identical ~40-line preamble to fetch the underlying estimator's per-class
probability matrix for their one-vs-rest multiclass paths. Extract it into
sklearn/_multiclass_proba.py (same pattern as _diagnosis_metrics.py) and
point all four tests at it.

Fix the class/column alignment while there: the preamble assumed
np.unique(dataset.y) matches predict_proba column order, but sklearn orders
columns by estimator.classes_ (training classes). Align on classes_ with a
np.unique fallback, binarize against the full training class list, and emit
per-class output only for classes present in the evaluated y. Net behavior
change: an evaluation slice missing a training class now computes metrics
for the present classes plus micro-average instead of raising SkipTestError.

Two guards the old width check provided implicitly are kept explicit: a
binary-trained model evaluated on a >2-class dataset skips (previously the
new alignment would have crashed with IndexError), and evaluation labels the
model was never trained on skip rather than being silently absorbed as
all-negative rows.

Adds unit tests for the helper (10) and un-gated sklearn multiclass tests
for all four call sites, including the missing-class scenario; the
pre-existing multiclass tests for ROC/PR were entirely xgboost-gated, so the
refactored paths now have coverage without the extra. PSI gains its first
unit test file. 114 passed / 5 xgboost-gated skips across
tests/unit_tests/model_validation.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@cachafla cachafla added the support Support-related PR label Jul 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request support Support-related PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants